Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate reflection free Jackson deserializers for classes without an empty constructor #45042

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

mariofusco
Copy link
Contributor

@mariofusco mariofusco commented Dec 10, 2024

Currently if a class doesn't have an empty constructor the generation of its reflection-free Jackson deserializer is skipped. This pull request is intended to overcome this limitation.

/cc @geoand

Copy link

quarkus-bot bot commented Dec 10, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit ee668b2.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 17 Windows

📦 extensions/resteasy-reactive/rest-client/deployment

io.quarkus.rest.client.reactive.stork.StorkResponseTimeLoadBalancerTest.shouldUseFasterService - History

  • expected: "hello, Alice" but was: "hello, I'm a slow server" - org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: 

expected: "hello, Alice"
 but was: "hello, I'm a slow server"
	at io.quarkus.rest.client.reactive.stork.StorkResponseTimeLoadBalancerTest.shouldUseFasterService(StorkResponseTimeLoadBalancerTest.java:58)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:513)
	at io.quarkus.test.QuarkusUnitTest.interceptTestMethod(QuarkusUnitTest.java:427)

@@ -112,7 +112,7 @@ public Dog echoDog(Dog dog) {
@POST
@Path("/record-echo")
@Consumes(MediaType.APPLICATION_JSON)
public StateRecord echoDog(StateRecord stateRecord) {
public StateRecord echoRecord(StateRecord stateRecord) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you perhaps missing something from the commit?
Asking because I don't see how this exercises the scenario this change is supposed to address

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method renaming is unrelated, it is just to fix the formerly wrong name that came from an excess of copy/paste :)

This endpoint and the test invoking it are actually exercising this improvement in the sense that returned StateRecord wasn't deserialized through the generated reflection-free deserializer before, since the record structurally doesn't have an no-args constructor. Now it generates the deserializers also for records. The final effect is of course the same (of course in a reflection-free way this time), the only issue is that I don't know how to check that the deserializers is actually generated and it is using it, since the net behaviour is the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks.

I don't have any suggestions for better testing at the moment

@geoand geoand merged commit 5d830e9 into quarkusio:main Dec 11, 2024
32 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Dec 11, 2024
@mariofusco mariofusco deleted the no-empty-ctor-deser branch December 11, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants